-
Notifications
You must be signed in to change notification settings - Fork 30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Keyboard accessibility improvements #138
Keyboard accessibility improvements #138
Conversation
Some initial thoughts after trying it out a bit:
|
Overall, though, everything is working pretty well and this is a huge improvement in keyboard usability! (I'll be away tomorrow through the 20th, but I can get on implementing the same for EPUB/snapshot once I'm back.) |
I agree. However, in future versions,
Good catches!
Yes, that's a good observation. Maybe in future versions it will debounce image annotation rerendering or pause it if
Sure.
No need to rush this. It’s still quite buggy and requires thorough testing, as we don’t want to disrupt Zotero at this stage. |
This is great! I think the keyboard shortcuts approach we took is quite intuitive and in general everything does work quite well. A few minor things I paid attention to after a bit of playing around:
I do have one question. Currently, once I have the new focus ring on the annotation, I have to press Enter for it to get selected for the resizing keyboard shortcuts to take effect. Isn't this somewhat close to the resizing mode that we decided against? Why is the selection of the annotation via Enter necessary in the first place? We tried to pick the shortcuts with Final observation here is that Enter on a focused underline or highlight annotation without a comment will also focus an empty comment field. Then, the first resizing shortcut keypress (e.g Shift-RightArrow) will just blur the comment field and the next keypress will do the resizing. It would be a bit hard to explain via a concise verbal announcement to a screen reader user this setup and I, personally, find it a bit confusing as well. If the resizing shortcut keypress were to select the annotation and begin moving it, we could know that the comment field does not need to be focused. Sorry if that last part got a bit too verbose in the end but hopefully it made sense! |
Thanks for the great insights!
Ok, I'll fix.
Good point, I'll fix.
I think Enter might be a bit too much here and we could keep it for more important tasks. Also:
Ok, I'll try to fix that.
However, if the annotation is selected with the mouse, you still need to be able to move it. Allowing movement in both modes (focus and selection) would be weird.
Now that annotation navigation with arrow keys is only possible from the annotation view, we can try using the arrow keys without a modifier, as you suggested. However, I'm a bit concerned it might make it too easy to move annotations, especially if the arrow keys will resize the right side of highlight/underline annotations.
I agree that it's confusing. However, if we use the arrow keys for moving, it won't work for selecting and moving the annotation. One option could be to do automatic focusing on the empty comment field only when the annotation is clicked with the mouse. But in that case, if you create an annotation with the keyboard and select it, you won't be able to quickly start typing into the comment field. |
That would totally be weird.
That is a fair point as well. We do want to make sure that these actions are very intentional... Would it make sense to select the focused annotation when either of those shortcuts is used? That may offer the benefits of not requiring an extra Enter keypress without making it likely that someone would change an annotation unintentionally and or having this functionality in both focused and selected modes?
I think this is actually a good idea, regardless which way we go with the keys used for annotation movements. While focusing the comment right away does offer the benefit of making adding a comment faster, I do think that the drawback of a less intuitive interface for screen readers may outweigh them. Predictability is an important factor, and the fact that sometimes selecting an annotation will focus a textfield and sometimes it won't is not necessarily predictable. If you don't know that this is how it works and rely on screen reader announcements, you wouldn't know which comment field you're in and why, so we would want to add more announcements to explain it, and I think it can get messy. I can't find a specific guidance that would be relevant here because this interface is quite complex and most WCAG examples are quite basic but change of context not explicitly caused by the user is generally a bad thing (e.g. in Tools > Developer > Translator Editor, switching between tabs will also focus the first element with each tab, and it was marked as a problem by the VPAT review). And in this case, we always select and focus the annotation and, in addition to that, move focus into the textfield, so I think this example is somewhat relevant here. |
It depends on whether we decide to use the arrow keys without a modifier. If we do, the arrow keys will simply move the focus to another object, instead of selecting the annotation. |
After further thought, I made the following changes:
Please let me know your thoughts. |
This is great! This collection of shortcuts feels very good to me - arrows feel right to move objects around and Shift(+Cmd)+Arrows does fit well underline and highlight, since it's almost like text selection. Ghost outline and image flickering is also gone for me. So far, the only additional thought I have is if we should close the find popup after an annotation is created via shortcut as well? I imaging once you add the annotation, you're done with the search process and are ready to move on to the next step - add some comments/tags, or just keep reading. And, if you do want to edit the annotation after it's made, when you close the popup, the selected annotation gets un-selected, so you would have to navigate to find it again. I'll play around to see if I have any more thoughts! |
So, if there are no objections from anyone, I'll thoroughly test and merge the code. We can always make small adjustments in the future. The DOM view could also have this implemented later, possibly once we are certain users don’t object to the changes (@AbeJellinek). |
Works for me! |
Sounds great! |
- Using the TAB and arrow keys, any currenlty visible object in the PDF view can be selected, including annotations, internal/external links, and citations. - Create annotations using keyboard shortcuts alone. - Create highlight/underline annotations from the find popup (it was the only possible option). - Move and resize annotations using keyboard shortcuts. - Clicking an annotation in the document view no longer switches focus to the annotation sidebar. To navigate between annotations, the annotation list in the sidebar must be focused. - The Escape key now performs the single most important action at the time, rather than closing and deactivating everything at once. - Use Enter/Escape to focus or blur annotation comments in the sidebar. - When focusing the annotation sidebar using the keyboard, the last selected annotation is now selected, instead of always navigating to the first one. New keyboard shortcuts: - Move Note, Text, Image, Ink annotation: ArrowKeys - Resize Text, Image, Ink annotation: Shift-ArrowKeys - Resize Highlight/Underline annotation: Shift-ArrowKeys, Shift-Cmd-ArrowKeys (macOS), Shift-Alt-ArrowKeys (Windows, Linux) - Create Note, Text, Image annotation: Ctrl-Option/Alt-3/4/5 - Create Highlight/Underline annotation from text selection or find popup result: Ctrl-Option/Alt-1/2 zotero/zotero#4224
3bb5463
to
562d7c2
Compare
I'll get on implementing this for EPUB/snapshot if OK with you @mrtcode |
Awesome! There are some tiny conflicts in #137 now (nothing major). I assume it's best for me wait for the EPUB before resolving, in case we have some additional conflicts there. |
Oh, forgot about
I'll still do a little initial work, but I think that's reasonable. |
Remove logic constructing a11y announcements from reader.ftl. Instead, reader will fetch and put together components as needed on the fly. Followup to zotero@cde21ac per zotero#4752 (comment) Also, add { command-or-alt } to zotero.ftl and use it for textual annotations instructions. On mac, the end of highlight and underline annotations is resized via Shift+Command+arrows, which is different from the Options modifier used in all other instances (per zotero/reader#138 (comment)) Finally, tweak aria-description of "Find in document" as it sounded as if Control+Option/Alt+1 would create either highlight or underline annotation.
Remove logic constructing a11y announcements from reader.ftl. Instead, reader will fetch and put together components as needed on the fly. Follow-up to cde21ac per #4752 (comment) Also, add { general-key-command } and { general-key-alt } and add a reader string that uses those on macOS and non-macOS for textual annotation instructions. On macOS, the end of highlight and underline annotations is resized via Shift+Command+arrows, which is different from the Option modifier used in all other instances (per zotero/reader#138 (comment)) Finally, tweak aria-description of "Find in Document" as it sounded as if Control+Option/Alt+1 would create either highlight or underline annotation.
Working on this for EPUB/snapshot, since it doesn't seem like we've gotten any complaints. Should Shift + Option/Alt + arrow keys resize highlights by word boundaries? That would track with the behavior on macOS (Option + arrow keys selects by word) and would be pretty convenient. I don't think it's too confusing that it "conflicts" with the text/image/ink shortcuts. |
Yes, because highlight resizing is conceptually similar to selection changes, it should support everything that a regular selection supports. In the future, this keyboard shortcut will also be supported in the PDF view. |
This PR implements the changes discussed in zotero/zotero#4224, but while doing that it also triggered other keyboard accessibility changes throughout the reader code. Once we agree on the changes and merge this PR, similar changes can be implemented in EPUB and snapshot views.
As discussed in zotero/zotero#4224:
Shift + Arrow Keys
Move note/text/image/ink annotation.Shift + Option/Alt + Arrow Keys
Resize text/image/ink annotation.Shift + Arrow Keys
Resize the right side of highlight/underline annotation.Shift + Cmd/Ctrl + Arrow Keys
Resize the left side of highlight/underline annotation.Ctrl + Option/Alt + 1/2
Create highlight/underline:Ctrl + Option/Alt + 3/4/5
Create note/text/image annotations.I used
Cmd/Ctrl
instead ofOption/Alt
for highlight/underline left side resizing because theOption
key is used for word selection on macOS, and resizing highlight/underline is essentially adjusting text selection (Update: this word selection mode currently doesn't work in PDF view text selection layer, but it will in future). However, should we consider doing the opposite and usingCmd/Ctrl
for resizing other annotations as well?Additional changes worth mentioning: